Repository navigation
feat(parse): accept explicit + sign in e-notation exponent - #248
Conversation
`1e+2`, `1.0e+2`, `1E+2` etc. now parse identically to `1e2`. `1e+` with no digits still rejects with MalformedExponentDigits. Fixes rainlanguage/rainlang#214. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
WalkthroughThis PR extends ChangesPositive Exponent Sign Support
Estimated code review effort: 1 (Trivial) | ~5 minutes Related PRs: None identified. Suggested labels: enhancement, tests Suggested reviewers: None identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/parse/LibParseDecimalFloat.sol (1)
141-151: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winMixed
-/+exponent signs silently mis-parsed instead of rejected.The new
+check runs unconditionally after the-skip, without verifying that no negative sign was already consumed. For input like"1e-+2":
- Line 145 skips the
-, advancingcursorto point at+.- Line 148 then matches
+at that position, advancescursorpast it, and overwriteseStartto point at2— discarding the previously consumed-entirely.- The exponent parses as
+2instead of being rejected, silently dropping the sign that was present in the input.Previously (before this PR)
"1e-+2"failed withMalformedExponentDigitsbecause+is not a digit. Now it succeeds with an incorrect (wrong-sign) exponent value. This is a real regression for a decimal-float parser that presumably feeds downstream numeric/financial logic.🐛 Proposed fix: only accept `+` when no `-` was already consumed
uint256 eStart = cursor; - cursor = LibParseChar.skipMask(cursor, end, CMASK_NEGATIVE_SIGN); - // Skip an optional explicit positive sign (e.g. 1e+2). Advance - // eStart past it so the int parser only sees the digit string. - if (LibParseChar.isMask(cursor, end, CMASK_PLUS_SIGN) == 1) { - cursor++; - eStart = cursor; - } + uint256 cursorBeforeSign = cursor; + cursor = LibParseChar.skipMask(cursor, end, CMASK_NEGATIVE_SIGN); + // Skip an optional explicit positive sign (e.g. 1e+2), but only + // if a negative sign was not already consumed above. Advance + // eStart past it so the int parser only sees the digit string. + if (cursor == cursorBeforeSign && LibParseChar.isMask(cursor, end, CMASK_PLUS_SIGN) == 1) { + cursor++; + eStart = cursor; + }Please add test cases for
"1e-+2"and"0.0e-+1"assertingMalformedExponentDigits(or another rejection), to lock in the correct behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/parse/LibParseDecimalFloat.sol` around lines 141 - 151, The exponent-sign handling in LibParseDecimalFloat.parseDecimalFloat currently accepts a plus sign even after a minus has already been consumed, which lets mixed signs like 1e-+2 slip through. Update the exponent parsing logic around the eValue/cursor/eStart block so the optional CMASK_PLUS_SIGN is only recognized when no CMASK_NEGATIVE_SIGN was skipped, and otherwise leave the parser to fail with MalformedExponentDigits. Add regression tests covering 1e-+2 and 0.0e-+1 to ensure mixed signs are rejected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/lib/parse/LibParseDecimalFloat.sol`:
- Around line 141-151: The exponent-sign handling in
LibParseDecimalFloat.parseDecimalFloat currently accepts a plus sign even after
a minus has already been consumed, which lets mixed signs like 1e-+2 slip
through. Update the exponent parsing logic around the eValue/cursor/eStart block
so the optional CMASK_PLUS_SIGN is only recognized when no CMASK_NEGATIVE_SIGN
was skipped, and otherwise leave the parser to fail with
MalformedExponentDigits. Add regression tests covering 1e-+2 and 0.0e-+1 to
ensure mixed signs are rejected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 12c3e737-fa22-481d-a6a1-9aceddbfddd3
📒 Files selected for processing (2)
src/lib/parse/LibParseDecimalFloat.soltest/src/lib/parse/LibParseDecimalFloat.t.sol
Regenerate DecimalFloat.json artifact after adding explicit + sign support in e-notation exponent parsing. Co-Authored-By: Claude <noreply@anthropic.com>
…[3b-attempt] ZOLTU_DEPLOYED_DECIMAL_FLOAT_ADDRESS and DECIMAL_FLOAT_CONTRACT_HASH are now set to the values computed from this PR's bytecode (explicit + sign in e-notation exponent changes source). Computed by testDeployAddress and testExpectedCodeHashDecimalFloat failure output: address: 0x9b0eF7FbDF04b3A58989860Dea0534C978d64464 hash: 0x369ac78d686dfdf1d41771dc7bf321dcd339b0a454dc9cf6c37eff579c3a0c3a Co-Authored-By: Claude <noreply@anthropic.com>
|
Producer note: WAITING-DEPLOY. Deploy constants now regenerated to this PR's bytecode (testDeployAddress/testExpectedCodeHash pass); the remaining red is the 5 testProdDeployment* network pins ('DecimalFloat not deployed') — same condition as main, needs the routine interactive pre-merge Zoltu deploy from this branch, not a code fix. |
|
🤖 ai:producer |
thedavidmeister
added
the
ai:needs-work
|
🤖 ai:vetter |
thedavidmeister
added
ai:blocked-on
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ve-exponent-sign # Conflicts: # crates/float/abi/DecimalFloat.json # src/lib/deploy/LibDecimalFloatDeploy.sol
|
@coderabbitai review |
|
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
1 participant
Refs rainlanguage/rainlang#214.
What
LibParseDecimalFloat.parseDecimalFloatInlineaccepts an explicit+aftere/E, so1e+2,1.0e+2and1E+2parse the same as1e2.An exponent takes one sign:
+is accepted only where-would be, never after it.1e-+2and1e+-2reject withMalformedExponentDigits, as does1e+with no digits (same as1e-).Tests
testParseLiteralDecimalFloatPositiveExponentSign: nine accepted forms, including signed coefficients and decimals.testParseLiteralDecimalFloatPositiveENoDigits:1e+and0.0e+reject.testParseLiteralDecimalFloatExponentSignAfterNegativeSign/...NegativeSignAfterPositiveSign:1e-+2and1e+-2reject. An earlier revision of this PR parsed1e-+2as1e2.Scope after the library/deploy split
mainwas merged in. The deploy constants and ABI artifact this PR used to touch now live inrain.math.float.deploy; those changes were dropped. No redeploy is involved. The diff is the parser and its tests only.🤖 Generated with Claude Code